Skip to content

Add Laya request preprocessing - #43

Open
linear3735 wants to merge 5 commits into
ThinkFlowLab:mainfrom
linear3735:codex/laya-input
Open

linear3735 wants to merge 5 commits into
ThinkFlowLab:mainfrom
linear3735:codex/laya-input

Conversation

@linear3735

@linear3735 linear3735 commented Sep 30, 2026 •

Copy link
Copy Markdown

Purpose

Pack English Laya requests into token rows, option-marker positions, question types and usage counts. Preserve question and option order and apply Laya 0.3.20's truncation rules.

#21 is merged. This PR adds preprocessing on top of that checkpoint layer: 496 core/configuration lines, excluding separate tests, docs and Cargo.lock. Refs #14.

Also preserve Cua-S1 JSON number parsing and Python float formatting when Laya enables serde_json/arbitrary_precision. Without this fix, three existing Cua-S1 tests fail in the combined workspace.

Fix private Number/RawValue key collisions in structured Laya inputs. Native callers now use Request::from_json(&str) or Request::from_value(Value); generic Deserialize is removed, while public fields and Serialize remain. New tests and helpers, including the Cua-S1 regression, live under repository-root tests/.

Test Plan

Run workspace tests, fmt, strict Clippy and a release build. Compare packed inputs against the frozen 17-case Laya reference. Check private object keys, numeric preservation, request fields and JSON/value depth boundaries. Pinned inputs and commands are in the CPU packing recipe.

System1-Omni Version / Commit: 37736f0. Original preprocessing increment: 219808b → 99c1d12.

Test Result

  • Local workspace and current CPU CI: 39 tests passed and 6 external-data/GPU tests were skipped. The official 17-case packing comparison passed separately; private-key regression tests failed before the fix and passed after it.
  • Earlier Cua-S1 checks passed all 7 unit tests both with and without arbitrary_precision; the production parser is unchanged by the test relocation.
  • fmt, strict Clippy, release build and strict MkDocs build passed locally.
  • Earlier full-checkpoint checks passed; checkpoint code is unchanged. No GPU inference or model-quality tests were run for this update.

CI for 37736f0: Rust CI, Docs build, benchmark harness tests passed.

Self-review

Before marking this PR ready for review or requesting maintainer review, complete
the self-review checklist.
Keep the PR in draft while this work is incomplete.
For agent assistance, use the optional precheck-pr skill.

  • I have reviewed the full diff and addressed the issues I found.
  • I have checked that the change follows the project's architecture and stays focused on the stated purpose.
  • I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
  • I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.

@linear3735 linear3735 mentioned this pull request Sep 30, 2026
4 tasks
@linear3735
linear3735 marked this pull request as ready for review September 30, 2026 02:48

@hsliuustc0106 hsliuustc0106 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent local review — Laya request preprocessing

Verdict: approved. Best-in-batch architecture conformance.

src/models/laya/src/preprocess.rs as a standalone processor module — token rows, option-marker positions, question types, usage counts, Laya 0.3.20 truncation rules, question/option order preserved — is precisely the processor placement the current contracts describe (prepare separate from execution). Tests live at repo-root tests/laya/ and the PR wires its test targets into ci.yml (the right pattern; see my #25 note). Locally at head 37736f08: laya suite passes with clippy -D warnings clean (25 passed / 3 checkpoint-gated ignored across the stack). CI green.

One rebase hazard flagged for the maintainer: this PR and #44 also modify cua_s1/native/src/json.rs (+44/-11) — the Python-compatible JSON module that has since moved to src/models/qwen3_5/native/src/json.rs and is contract-sensitive for Cua-S1. That hunk needs careful re-application against the moved module, with the cua_s1 JSON contract tests as the gate.

Approval per the repo review process; reflects head 37736f08 only.

@hsliuustc0106 hsliuustc0106 mentioned this pull request Oct 5, 2026
2 of 4 tasks
@hsliuustc0106

Copy link
Copy Markdown
Contributor

fix conflcits

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants